Skip to content

Refactor mirror/get_mirrored to fail fast and remove exitstat masking#82

Merged
patritzenfeld merged 6 commits into
fmiduefrom
copilot/refactor-mirror-get-mirrored
May 22, 2026
Merged

Refactor mirror/get_mirrored to fail fast and remove exitstat masking#82
patritzenfeld merged 6 commits into
fmiduefrom
copilot/refactor-mirror-get-mirrored

Conversation

Copy link
Copy Markdown

Copilot AI commented May 21, 2026

mirror/get_mirrored previously accumulated failures in exitstat, which masked control-flow errors and incorrectly reported failure on fresh mirror directories. This change shifts the script to immediate-exit behavior and removes the false-failure path called out in review.

  • Fail-fast error handling

    • Removed exitstat tracking and final deferred exit.
    • Converted operational failures (touch, cleanup, wget, hash append, final checksum validation) to immediate exit 1.
  • Correct stale-file cleanup control flow

    • Replaced && ... || exitstat=1 chain with an explicit existence guard so missing files on first run are not treated as errors.
    • Cleanup now fails only when cleanup itself fails.
  • Behavioral result

    • Fresh mirror directories no longer trigger a spurious non-zero exit.
    • Real failures stop the script at the point of failure instead of continuing with partially valid state.
if [ -e "${file}" ]; then
    find "${file}" -type f -mtime "+${MAX_DAYS}" -print0 | xargs -0 -r rm || exit 1
fi

[ -f "${file}" ] || \
  wget --no-verbose --no-use-server-timestamps \
       --directory-prefix="${mirror_dir}" --force-directories "${url}" || \
  exit 1

Copilot AI linked an issue May 21, 2026 that may be closed by this pull request
Agent-Logs-Url: https://github.com/fmidue/codeworld/sessions/ac325de1-aed8-4f47-a616-cea1987f0307

Co-authored-by: jvoigtlaender <5853832+jvoigtlaender@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor mirror/get_mirrored to handle failures properly Refactor mirror/get_mirrored to fail fast and remove exitstat masking May 21, 2026
Copilot AI requested a review from jvoigtlaender May 21, 2026 14:12
@jvoigtlaender
Copy link
Copy Markdown
Member

@copilot, check compatibility of the recent merge from main with your own refactorings here.

Copy link
Copy Markdown
Author

Copilot AI commented May 21, 2026

@copilot, check compatibility of the recent merge from main with your own refactorings here.

Checked on 5a91e32: the merge from main is compatible with the fail-fast refactor. I re-ran targeted script validation (bash -n plus mocked success/failure runs) and confirmed expected behavior (fresh mirror path succeeds, download failure exits immediately). Latest CI runs on this branch after the merge are also green.

@jvoigtlaender jvoigtlaender marked this pull request as ready for review May 21, 2026 15:53
Copilot AI review requested due to automatic review settings May 21, 2026 15:53
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors mirror/get_mirrored to fail fast on operational errors and removes the prior exitstat accumulation that could mask control-flow mistakes and incorrectly report failure on a fresh mirror directory.

Changes:

  • Removed deferred exitstat tracking in favor of immediate exit 1 on failures (e.g., checksum append, final validation).
  • Fixed stale-file cleanup so missing files on a first run aren’t treated as an error.
  • Ensured touch of the cached hash file fails the script if it cannot be created.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@patritzenfeld
Copy link
Copy Markdown
Member

I'll have to update some of the download paths and/or file hashes before we merge this.

Before, the hash mismatches of downloaded files were ignored, but that doesn't happen now. Also, there are some downloads referencing "latest" versions, which makes no sense if we compare to a frozen file hash.

@patritzenfeld patritzenfeld merged commit 1af6430 into fmidue May 22, 2026
2 checks passed
@patritzenfeld patritzenfeld deleted the copilot/refactor-mirror-get-mirrored branch May 22, 2026 14:33
This was referenced May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor mirror/get_mirrored

4 participants